trusted-execution-clusters: Enable full suite - #82662
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe Azure integration test now uses cached CI inputs, Azure credentials, explicit resource settings, and repeated ChangesAzure integration test
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
approved
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh (1)
10-10: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the Azure CLI dependency.
pip install azure-cliresolves the package and all transitive dependencies at job runtime, allowing unrelated releases to change or break this test. Use a reviewed constraint/lock file with hashes or bake the pinned version intoazure-test-image.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh` at line 10, Update the Azure CLI installation command in the trusted execution cluster test setup to use a reviewed pinned dependency source, such as a constraints/lock file with hashes, or replace the runtime install with the pinned Azure CLI version from azure-test-image. Ensure transitive dependencies are reproducibly constrained rather than resolved freely at job runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`:
- Around line 40-43: Update the attestation test execution around the make
attestation-tests loop to avoid unbounded retries and ensure a final test
failure makes the step exit non-zero. Prefer running make attestation-tests once
for normal CI, or implement a bounded retry with an explicit failure exit after
the retry limit.
- Around line 28-30: Remove the `git remote add test`, `git fetch test`, and
`git switch timed-client-ci` overrides so normal CI runs the reviewed checkout;
also remove the mutable personal Quay image override at the corresponding
image-selection symbols. If debugging support is required, isolate it behind an
explicit manual path and pin the source revision by commit and image by digest.
- Line 5: Replace the direct curl-to-shell invocation with a pinned or prebuilt
Rust toolchain in the trusted execution setup. If retaining the installer,
download it to a local file, verify its reviewed checksum or signature, and only
then execute it; ensure no unverified remote content runs with access to Azure
secrets.
---
Nitpick comments:
In
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`:
- Line 10: Update the Azure CLI installation command in the trusted execution
cluster test setup to use a reviewed pinned dependency source, such as a
constraints/lock file with hashes, or replace the runtime install with the
pinned Azure CLI version from azure-test-image. Ensure transitive dependencies
are reproducibly constrained rather than resolved freely at job runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 77d53dd0-d4ab-4b7e-a12b-3bbd7a72d084
⛔ Files ignored due to path filters (1)
ci-operator/jobs/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (6)
ci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main.yamlci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main__azure.yamlci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/OWNERSci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.shci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-ref.metadata.jsonci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-ref.yaml
|
|
||
| set -euo pipefail | ||
|
|
||
| curl https://sh.rustup.rs | sh -s -- -y |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
# Locate and inspect the target command script and nearby context.
target="ci-operator/step-registry/${PWD##*/}/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh"
if [ -f "$target" ]; then
wc -l "$target"
sed -n '1,80p' "$target" | cat -n
else
echo "Target not found: $target"
fi
echo '--- matching command scripts ---'
fd -a 'trusted-execution-clusters-ref-azure-integration-test-commands.sh' . || true
echo '--- rustup/azure credential usage ---'
rg -n 'curl .*rustup|rustup|azure|AZURE|[A-Z_]*SECRET|/tmp/secrets|ssh-agent|SHARED_DIR' ci-operator/step-registry 2>/dev/null | head -200Repository: openshift/release
Length of output: 36470
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target command script and nearby context.
target="ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh"
if [ -f "$target" ]; then
echo "== file stats =="
wc -l "$target"
echo "== target content =="
sed -n '1,120p' "$target" | cat -n
else
echo "Target not found: $target"
fi
echo '--- matching command scripts ---'
fd -a 'trusted-execution-clusters-ref-azure-integration-test-commands.sh' . || true
echo '--- rustup/azure credential usage ---'
rg -n 'curl .*rustup|rustup|azure|AZURE|[A-Z_]*SECRET|/tmp/secrets|ssh-agent|SHARED_DIR' ci-operator/step-registry 2>/dev/null | head -200Repository: openshift/release
Length of output: 35715
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target command script and nearby context.
target="ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh"
if [ -f "$target" ]; then
echo "== file stats =="
wc -l "$target"
echo "== target content =="
sed -n '1,120p' "$target" | cat -n
else
echo "Target not found: $target"
echo "candidate files:"
fd -a 'trusted-execution-clusters-ref-azure-integration-test-commands.sh' . || true
fi
echo '--- rustup/azure credential usage ---'
rg -n 'curl .*rustup|rustup|azure|AZURE|[A-Z_]*SECRET|/tmp/secrets|ssh-agent|SHARED_DIR' ci-operator/step-registry 2>/dev/null | head -200Repository: openshift/release
Length of output: 37951
Do not pipe an unverified remote script into the shell.
Line 5 executes mutable remote content directly before Azure credentials are loaded, and the script runs with access to /tmp/secrets/azure. Use a pinned/prebuilt Rust toolchain instead, or download the installer and verify a reviewed checksum/signature before executing the local file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`
at line 5, Replace the direct curl-to-shell invocation with a pinned or prebuilt
Rust toolchain in the trusted execution setup. If retaining the installer,
download it to a local file, verify its reviewed checksum or signature, and only
then execute it; ensure no unverified remote content runs with access to Azure
secrets.
Source: Linters/SAST tools
| git remote add test https://github.com/Jakob-Naucke/trusted-cluster-operator | ||
| git fetch test | ||
| git switch timed-client-ci |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Do not run a mutable personal fork with CI cloud credentials.
Lines 28-30 replace the PR checkout with Jakob-Naucke/trusted-cluster-operator:timed-client-ci. Azure authentication has already occurred on Lines 21-24, so make attestation-tests executes unreviewed fork code with cloud authentication. Lines 32-33 also select a mutable personal Quay image instead of a CI-built immutable artifact. Remove these overrides for normal CI; keep debugging branches/images isolated behind an explicit manual path and pin any required revision by commit/digest.
Also applies to: 32-33
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`
around lines 28 - 30, Remove the `git remote add test`, `git fetch test`, and
`git switch timed-client-ci` overrides so normal CI runs the reviewed checkout;
also remove the mutable personal Quay image override at the corresponding
image-selection symbols. If debugging support is required, isolate it behind an
explicit manual path and pin the source revision by commit and image by digest.
| echo "[INFO] Running attestation tests until failure..." | ||
| while make attestation-tests | ||
| do : | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make failures fail the step and bound the loop.
With make in the while condition, set -e does not propagate its failure. After any successful iteration, the final : makes the loop return zero when make eventually fails; if tests always pass, the job runs indefinitely. Remove the loop for normal CI or use a bounded retry loop so failures exit non-zero.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`
around lines 40 - 43, Update the attestation test execution around the make
attestation-tests loop to avoid unbounded retries and ensure a final test
failure makes the step exit non-zero. Prefer running make attestation-tests once
for normal CI, or implement a bounded retry with an explicit failure exit after
the retry limit.
0f6b409 to
13cb31e
Compare
|
Infinite loop was successful until timeout. Promising. Let's give it another go including cluster-level tests, which I forgot earlier. /pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
adding up, 4 good runs in a row (failure is from timeout) |
13cb31e to
21c4b81
Compare
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
21c4b81 to
fd1a012
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh (1)
10-10: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPin the Azure CLI dependency or bake it into
azure-test-image.
pip install azure-cliresolves the latest package at job runtime. This makes startup depend on current PyPI contents and network availability. Install a tested version in the image or use a committed constraints file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh` at line 10, Update the Azure CLI installation in the trusted execution cluster test setup to use a pinned, tested version rather than resolving the latest package at runtime. Prefer relying on the preinstalled version in azure-test-image or install via a committed constraints/version specification while preserving the existing Azure CLI availability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main__azure.yaml`:
- Around line 39-53: Regenerate the Azure Prow job using `make update` so the
generated configuration no longer references the removed Dockerfiles. Commit all
generated changes, including updates to
`trusted-execution-clusters-operator-main__azure.yaml`, and do not manually edit
`zz_generated_metadata`.
---
Nitpick comments:
In
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`:
- Line 10: Update the Azure CLI installation in the trusted execution cluster
test setup to use a pinned, tested version rather than resolving the latest
package at runtime. Prefer relying on the preinstalled version in
azure-test-image or install via a committed constraints/version specification
while preserving the existing Azure CLI availability.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 02ec237c-6eef-4561-a983-19579820fd71
⛔ Files ignored due to path filters (1)
ci-operator/jobs/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (6)
ci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main.yamlci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main__azure.yamlci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/OWNERSci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.shci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-ref.metadata.jsonci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/OWNERS
- ci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main.yaml
- ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-ref.metadata.json
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
fd1a012 to
7875c8b
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jakob-Naucke The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
nice, ran successfully until timeout, again /pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
7875c8b to
bb8e1eb
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
In hindsight, "success" is a potential failure because the loop exited. Added an exit. Investigating before rerehearsing. |
Following up on openshift#79393. Debug first. Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
bb8e1eb to
f2e27ed
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
One new failure but let's go on a bit /pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
bringup failed /pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@Jakob-Naucke: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Following up on #79393. Debug first.
Summary by CodeRabbit
trusted-execution-clusterscomponent.make integration-testsruns.trusted-cluster-operatorimages and configures registry, image tag, test threads, and a higher timeout multiplier.